-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add GuidedChoice to ChatCompletionRequest #1034
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add GuidedChoice to ChatCompletionRequest #1034
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1034 +/- ##
===========================================
+ Coverage 86.30% 99.59% +13.29%
===========================================
Files 43 34 -9
Lines 2387 2205 -182
===========================================
+ Hits 2060 2196 +136
+ Misses 302 6 -296
+ Partials 25 3 -22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@AyushSawant18588 thank you for the PR! Let's rename this field to |
So like chat_template_kwargs this is an extra_body field, hence its supported by OpenAI as well and is not vllm specific. For example:
|
@sashabaranov wanted to confirm, are the changes made in this PR fine or should I update? |
@AyushSawant18588 okay, I see the issue! I've just merged #906, let's introduce |
@sashabaranov most of the fields we use are on top level and since this field is widely used for the use case of structured outputs can we merge this PR? |
@AyushSawant18588 but it's not part of official OpenAI API right? If it is, could you please point me to the documentation? |
@AyushSawant18588 we can make it |
a840a3c
to
e893d0d
Compare
@sashabaranov do these changes look good? made NonOpenAIExtensions.GuidedChoice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for the guided_choice
parameter in chat completions by embedding a new extensions struct into the request model.
- Introduce
NonOpenAIExtensions
struct with aGuidedChoice
field - Embed
NonOpenAIExtensions
anonymously intoChatCompletionRequest
- Align JSON tags to include
guided_choice
in serialized output
Comments suppressed due to low confidence (2)
chat.go:254
- Add unit tests to ensure
GuidedChoice
is correctly marshaled and unmarshaled in JSON, verifying it appears underguided_choice
when set and is omitted when empty.
GuidedChoice []string `json:"guided_choice,omitempty"`
chat.go:251
- Update the public API documentation or README to mention the new
guided_choice
parameter and link to the relevant usage examples or tests.
// NonOpenAIExtensions contains non-standard OpenAI API extensions.
chat.go
Outdated
@@ -309,6 +315,8 @@ type ChatCompletionRequest struct { | |||
ChatTemplateKwargs map[string]any `json:"chat_template_kwargs,omitempty"` | |||
// Specifies the latency tier to use for processing the request. | |||
ServiceTier ServiceTier `json:"service_tier,omitempty"` | |||
// Embedded struct for non-OpenAI extensions | |||
NonOpenAIExtensions `json:",inline"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The json:",inline"
tag is not recognized by Go’s encoding/json
and can be removed, since anonymous struct fields are inlined by default.
NonOpenAIExtensions `json:",inline"` | |
NonOpenAIExtensions |
Copilot uses AI. Check for mistakes.
@AyushSawant18588 thank you for the update, this seems like a pretty elegant solution! Here are a few things to consider:
|
@sashabaranov Can you check the changes? I renamed struct to ChatCompletionRequestExtensions cause name was getting too big but let me know if it has to be changed |
Describe the change
Adds guided_choice param support to chat completions
Provide OpenAI documentation link
Structured output vLLM support